feat: Add option to place sidebar above main content on mobile #1088
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #1010
This PR adds a new
"always-above"
option for themobile
component ofsidebar(open=)
, in other words:This feature works by:
sidebar_open_on()
that"always-above"
is a valid choice for themobile
component.main
andsidebar
in the markup whenopen$mobile == "always-above"
.This won't change visual order for screens above mobile size, where the layout is determined by
grid
. It will, however, result in the sidebar receiving tab focus before the main content area, across all screen sizes. That seems a reasonable trade-off and in-line with the types of UIs where users would want the sidebar to be shown before the main content on mobile devices.Fixes #1081
Incidentally, I ran into the problem described in #1081 while working on this. The issue turns out to be that we manually provide values to
rlang::arg_match()
, so callingsidebar_open_on()
without arguments could potentially throw. The fix is to use scalar default values in that helper function.